Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[AMORO-3180] Implement health score calculation for tables #3181

Merged
merged 1 commit into from
Sep 9, 2024
Merged

[AMORO-3180] Implement health score calculation for tables #3181

merged 1 commit into from
Sep 9, 2024

Conversation

MarigWeizhi
Copy link
Contributor

Why are the changes needed?

Close #3180.

Brief change log

  • Implement health score calculation for tables
  • Added TABLE_SUMMARY_HEALTH_SCORE metric in the TableSummaryMetrics
  • Show health score on the table detail page

How was this patch tested?

  • Add some test cases that check the changes thoroughly including negative and positive cases if possible

  • Add screenshots for manual tests if appropriate
    image

  • Run test locally before making a pull request

Documentation

  • Does this pull request introduce a new feature? (yes )
  • If yes, how is the feature documented? (not documented)

@github-actions github-actions bot added type:docs Improvements or additions to documentation module:ams-server Ams server module module:ams-dashboard Ams dashboard module module:common labels Sep 7, 2024
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.

Project coverage is 22.49%. Comparing base (8207efc) to head (677a01f).
Report is 10 commits behind head on master.

Files with missing lines Patch % Lines
...rg/apache/amoro/table/descriptor/TableSummary.java 0.00% 4 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (8207efc) and HEAD (677a01f). Click for more details.

HEAD has 2 uploads less than BASE
Flag BASE (8207efc) HEAD (677a01f)
core 2 0
Additional details and impacted files 
@@             Coverage Diff              @@
##             master    #3181      +/-   ##
============================================
- Coverage     29.43%   22.49%   -6.95%     
+ Complexity     3685     2283    -1402     
============================================
  Files           568      379     -189     
  Lines         47267    38030    -9237     
  Branches       6196     5442     -754     
============================================
- Hits          13912     8553    -5359     
+ Misses        32372    28740    -3632     
+ Partials        983      737     -246
Flag Coverage Δ
core ?
trino 22.49% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

zhoujinsong
zhoujinsong reviewed Sep 9, 2024
View reviewed changes
Copy link
Contributor

@zhoujinsong zhoujinsong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MarigWeizhi

Thanks for the contribution, I left some comments, PTAL.

amoro-ams/src/main/java/org/apache/amoro/server/optimizing/plan/CommonPartitionEvaluator.java Show resolved Hide resolved
amoro-ams/src/main/java/org/apache/amoro/server/optimizing/plan/CommonPartitionEvaluator.java Show resolved Hide resolved
amoro-ams/src/main/java/org/apache/amoro/server/optimizing/plan/CommonPartitionEvaluator.java Show resolved Hide resolved
amoro-ams/src/main/java/org/apache/amoro/server/optimizing/plan/CommonPartitionEvaluator.java Show resolved Hide resolved
zhoujinsong
zhoujinsong approved these changes Sep 9, 2024
View reviewed changes
Copy link
Contributor

@zhoujinsong zhoujinsong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@zhoujinsong zhoujinsong merged commit bc2e33e into apache:master Sep 9, 2024
4 checks passed
@MarigWeizhi MarigWeizhi deleted the health branch September 10, 2024 03:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zhoujinsong zhoujinsong zhoujinsong approved these changes

Assignees
No one assigned
Labels
module:ams-dashboard Ams dashboard module module:ams-server Ams server module module:common type:docs Improvements or additions to documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Subtask]: Implement health score calculation for tables
3 participants
@MarigWeizhi @codecov-commenter @zhoujinsong